Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Expose named exports for ESM build #2223

Merged
merged 1 commit into from
Oct 6, 2021
Merged

Expose named exports for ESM build #2223

merged 1 commit into from
Oct 6, 2021

Conversation

benmccann
Copy link
Contributor

Marked version:

Tested against 3.0.4

Markdown flavor: all

Description

Currently, you can only used named exports from the CJS build. This will write them in such a way that Rollup can figure out they're exposed and should be exported in the ESM build as well.

Expectation

I should be able to do import { parse } from 'marked';. I expect lib/marked.esm.js to export the parse method

Result

Today I get:

'parse' is not exported by 'node_modules/.pnpm/marked@3.0.4/node_modules/marked/lib/marked.esm.js'

That's because lib/marked.esm.js only has a default export

What was attempted

I tried to do import { parse } from 'marked';

Contributor

  • Test(s) exist to ensure functionality and minimize regression (if no tests added, list tests covering this PR); or,
  • no tests required for this PR.
  • If submitting new feature, it has been documented in the appropriate places.

Committer

In most cases, this should be a different person than the contributor.

@vercel
Copy link

vercel bot commented Oct 5, 2021 •
edited
Loading

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/markedjs/markedjs/Gv3eqhvsrkpvPKFwfsounN2FhTLf
✅ Preview: https://markedjs-git-fork-benmccann-named-exports-markedjs.vercel.app

@Conduitry
Copy link

Would it be a good idea to add a pkg.exports map at the same time, so that native Node ESM consumers (not using a bundler) will also get the ESM version rather than the CJS version?

@benmccann
Copy link
Contributor Author

There's a pending PR for it: #1725. Maybe not a bad idea, but I'd prefer to do it in a separate PR to keep this one simpler

@Conduitry
Copy link

Yup, makes sense.

@benmccann
Copy link
Contributor Author

Thanks for the approval @UziTech! Just curious, is there anything else needed before this can be merged?

@UziTech
Copy link
Member

UziTech commented Oct 6, 2021

I think this looks good. On marked we want 2 approvals for each PR so we are just waiting on someone else in the markedjs org to approve it.

@UziTech UziTech merged commit 3959651 into markedjs:master Oct 6, 2021
@benmccann benmccann deleted the named-exports branch October 6, 2021 20:33
github-actions bot pushed a commit that referenced this pull request Oct 6, 2021
@semantic-release-bot
chore(release): 3.0.5 [skip ci]
6c65489 
## [3.0.5](v3.0.4...v3.0.5) (2021-10-06)

### Bug Fixes

* Expose named exports for ESM build ([#2223](#2223)) ([3959651](3959651))
@github-actions
Copy link

github-actions bot commented Oct 6, 2021

🎉 This PR is included in version 3.0.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

@splch
Copy link

splch commented Oct 6, 2021

This update broke my website's markdown renderer. I'm getting a "marked.setOptions is not a function" error.
https://slc.is

UziTech added a commit to UziTech/marked that referenced this pull request Oct 6, 2021
@UziTech
revert markedjs#2223
6e5758d
@UziTech UziTech mentioned this pull request Oct 6, 2021
Closed
@UziTech
Copy link
Member

UziTech commented Oct 6, 2021

it seems like this change changed lib/marked.js into an ES Module

@Conduitry
Copy link

https://unpkg.com/marked@3.0.5/lib/marked.js It certainly doesn't look like that's what happened.

@UziTech
Copy link
Member

UziTech commented Oct 6, 2021

Object.defineProperty(exports, '__esModule', { value: true });

@UziTech
Copy link
Member

UziTech commented Oct 6, 2021

@splch you should set a version on your dependencies so they don't break automatically.

@splch
Copy link

splch commented Oct 6, 2021

For sure, I just like using the latest for development purposes so I tend to leave them unspecified.

@benmccann benmccann mentioned this pull request Oct 6, 2021
Merged
5 tasks
@benmccann
Copy link
Contributor Author

Here's a fix for the issue: #2226

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@UziTech UziTech UziTech approved these changes

@styfle styfle styfle approved these changes

@calculuschild calculuschild Awaiting requested review from calculuschild

@davisjam davisjam Awaiting requested review from davisjam

@joshbruce joshbruce Awaiting requested review from joshbruce

Assignees
No one assigned
Labels
hacktoberfest-accepted released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@benmccann @Conduitry @UziTech @splch @styfle